Skip to content

[enhancement](cloud) add bvar to monitor s3 throughput&QPS#34087

Merged
dataroaring merged 2 commits intoapache:masterfrom
freemandealer:s3bvar
Apr 25, 2024
Merged

[enhancement](cloud) add bvar to monitor s3 throughput&QPS#34087
dataroaring merged 2 commits intoapache:masterfrom
freemandealer:s3bvar

Conversation

@freemandealer
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@freemandealer
Copy link
Contributor Author

run buildall

dataroaring
dataroaring previously approved these changes Apr 24, 2024
Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Apr 24, 2024
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Apr 24, 2024
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

bvar::Adder<uint64_t> s3_file_reader_total("s3_file_reader", "total_num");
bvar::Adder<uint64_t> s3_bytes_read_total("s3_file_reader", "bytes_read");
bvar::Adder<uint64_t> s3_file_being_read("s3_file_reader", "file_being_read");
bvar::LatencyRecorder s3_bytes_per_read("s3_file_reader", "bytes_per_read"); // also QPS
Copy link
Contributor

@gavinchou gavinchou Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems calculating the rate of s3_bytes_read_total (throughput) and s3_file_reader_read_counter (qps), by grafana, will do the work.
Adding redundant metrics is a burden for the scraping procedure (Prometheus).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bvar has fine granularity, so it is easier to identify throughput issue (whether caused by long wait or low instantaneous speed). So I insist, but we can delete them when the optimization work is done.

Signed-off-by: freemandealer <freeman.zhang1992@gmail.com>
@freemandealer
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.19% (8915/25337)
Line Coverage: 26.96% (73292/271851)
Region Coverage: 26.14% (37867/144873)
Branch Coverage: 22.96% (19279/83972)
Coverage Report: http://coverage.selectdb-in.cc/coverage/475752af8743c98a91388e9e999a4db32b33c996_475752af8743c98a91388e9e999a4db32b33c996/report/index.html

@freemandealer
Copy link
Contributor Author

run cloudp1

@freemandealer
Copy link
Contributor Author

run cloud_p1

Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 25, 2024
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@dataroaring dataroaring merged commit 6736f02 into apache:master Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants